feat: adding support for multiqueue#650
Conversation
drew-viles
requested review from
65278,
mcbenjemaa,
pborn-ionos and
wikkyk
as code owners
TestsPlease note that running unit and e2e tests requires manual approval from a team member. e2e testsWe use labels to control which e2e tests contexts are run:
ℹ️ Ask a team member to add the requested labels if you don't have enough permissions. |
|
Nice and simple change but we'll hold it for now until after we have released 0.8.0 and v1alpha2. |
wikkyk
left a comment
wikkyk
left a comment
There was a problem hiding this comment.
Please port this to v1alpha2!
Remember to update the tests with the correct type, too.
|
|
Sorry absolutely slammed atm, I will pick this up again asap though |
|
v0.8.0 has been released so we are reopening feature merges. Please port your PR to v1alpha2 and address my comments and we'll do another review. This is a good feature, I would really like it to go in. |
|
I'll looks at getting it done this weekend. Time has been not been on my side the past few weeks. |
drew-viles
force-pushed
the
network-multiqueue
branch
from
28aa04f to
12f91d3
Compare
|
ok, I think I've got everything covered, |
wikkyk
force-pushed
the
network-multiqueue
branch
from
aafde41 to
609d7e0
Compare
drew-viles
force-pushed
the
network-multiqueue
branch
from
ce75146 to
d823356
Compare
|
ok, addressed those points. 🤞 Hopefully that's good to go now. |
wikkyk
force-pushed
the
network-multiqueue
branch
from
f5e007f to
da916fe
Compare
|
Nice, I really like it now. As this is a new feature and I'm intending for v0.8.2 to be primarily a bugfix release, I'll formally approve and schedule this for merging into v0.8.3. |
|
Edit: Thanks! |
|
I've just noticed all of my commits except the last one are unverified. I'm not sure why that happened tbh as I have signing on by default - something obviously got skipped when I did them 🤷 . If you want me to sign them all and re-push them let me know. Happy to do so! |
wikkyk
force-pushed
the
network-multiqueue
branch
from
3eb7ba0 to
9537071
Compare
|
It's because I rebased your commits, so technically they're new commits which you haven't signed. Also the plans for v0.8.2 have changed, merging this now pending test results. |
wikkyk
force-pushed
the
main
branch
3 times, most recently
from
89802a6 to
d6fde2d
Compare
Right now, users cannot add queues to the network NIC. With this PR, users will now be able to optionally supply a value into the CAPI provider to configure queues.
… max and nill queue value
wikkyk
force-pushed
the
network-multiqueue
branch
from
9537071 to
52e2264
Compare
|
|
Test coverage is lower than expected because the extractor is uncovered. (Actually, it just doesn't seem reachable but it is tested indirectly; were it not working, proxmoxmachine_types tests would fail.) I'm ok with this because of #653. |
|
Thanks! |
2 participants
Issue #, if available:
#501
Description of changes:
Right now, users cannot add queues to the network NIC. With this PR, users will now be able to optionally supply a value into the CAPI provider to configure queues.
Testing performed:
Added tests for
make testand ran with success. I'm currently trying to get access to a proxmox with shared storage as it appears to be required for this where as right now, I'm using local storage.If you want to wait for that testing to be done on my side I totally get it! However, the change seems pretty straight forward on the whole so I'll let you make the call from your side.